-
Notifications
You must be signed in to change notification settings - Fork 694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementation of the new validator disabling strategy #2226
Conversation
Handles validator disabling in the backing subsystem. The PR introduces two changes: 1. Don't import statements from disabled validators. 2. Don't validate and second if the local validator is disabled. The purpose of this change is first to ignore statements from disabled validators on the node side. This is an optimisation as these statements are supposed to be cleared in the runtime too (still not implemented at the moment). Additionally if the local node is a validator which is disabled - don't process any new candidates. --------- Co-authored-by: ordian <noreply@reusable.software> Co-authored-by: ordian <write@reusable.software>
* master: Removed TODO from test-case for hard-coded delivery fee estimation (#2042) Expose collection attributes from `Inspect` trait (#1914) `polkadot-parachain-primitives` should not depend on `frame-support`. (#1897) [testnet] Align testnet system parachain runtimes using `RelayTreasuryLocation` and `SystemParachains` in the same way (#2023) Sort the benchmarks before listing them (#2026) publish pallet-root-testing (#2017) Contracts: Add benchmarks to include files (#2022) Small optimisation to `--profile dev` wasm builds (#1851) basic-authorship: Improve time recording and logging (#2010) Application Crypto and BEEFY Support for paired (ECDSA,BLS) crypto (#1815) [ci] Run check-rust-feature-propagation in pr and master (#2012) Improve features dev-ex (#1831) Remove obsolete comment. (#2008)
And offenders list is already being cleared here:
|
But this is the state of offenders, not disabled state in session pallet. Although, it's currently cleared on session changes, so it's fine. |
Exactly. I had a concern why the validator indices are not recalculated but as far as I can see in the code, the active set is not reshuffled between sessions within an era. |
9e1039e
to
b16c8b6
Compare
…ge::ValidateFromExhaustive` now has got named fields
…n't disable if disabled validators will go beyond the byzantine threshold
The technical comments have been resolved and the PR is audited and tested
Closes #1966, #1963 and #1962. Disabling strategy specification [here](#2955). (Updated 13/02/2024) Implements: * validator disabling for a whole era instead of just a session * no more than 1/3 of the validators in the active set are disabled Removes: * `DisableStrategy` enum - now each validator committing an offence is disabled. * New era is not forced if too many validators are disabled. Before this PR not all offenders were disabled. A decision was made based on [`enum DisableStrategy`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/primitives/staking/src/offence.rs#L54). Some offenders were disabled for a whole era, some just for a session, some were not disabled at all. This PR changes the disabling behaviour. Now a validator committing an offense is disabled immediately till the end of the current era. Some implementation notes: * `OffendingValidators` in pallet session keeps all offenders (this is not changed). However its type is changed from `Vec<(u32, bool)>` to `Vec<u32>`. The reason is simple - each offender is getting disabled so the bool doesn't make sense anymore. * When a validator is disabled it is first added to `OffendingValidators` and then to `DisabledValidators`. This is done in [`add_offending_validator`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/slashing.rs#L325) from staking pallet. * In [`rotate_session`](https://github.com/paritytech/polkadot-sdk/blob/bdbe98297032e21a553bf191c530690b1d591405/substrate/frame/session/src/lib.rs#L623) the `end_session` also calls [`end_era`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/pallet/impls.rs#L490) when an era ends. In this case `OffendingValidators` are cleared **(1)**. * Then in [`rotate_session`](https://github.com/paritytech/polkadot-sdk/blob/bdbe98297032e21a553bf191c530690b1d591405/substrate/frame/session/src/lib.rs#L623) `DisabledValidators` are cleared **(2)** * And finally (still in `rotate_session`) a call to [`start_session`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/pallet/impls.rs#L430) repopulates the disabled validators **(3)**. * The reason for this complication is that session pallet knows nothing abut eras. To overcome this on each new session the disabled list is repopulated (points 2 and 3). Staking pallet knows when a new era starts so with point 1 it ensures that the offenders list is cleared. --------- Co-authored-by: ordian <noreply@reusable.software> Co-authored-by: ordian <write@reusable.software> Co-authored-by: Maciej <maciej.zyszkiewicz@parity.io> Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: command-bot <> Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
* StorageProofSize -> StorageSize * Rename benchmarks * StorageSize -> UnverifiedStorageProofParams * Fix clippy * Fix priority boost
Closes paritytech#1966, paritytech#1963 and paritytech#1962. Disabling strategy specification [here](paritytech#2955). (Updated 13/02/2024) Implements: * validator disabling for a whole era instead of just a session * no more than 1/3 of the validators in the active set are disabled Removes: * `DisableStrategy` enum - now each validator committing an offence is disabled. * New era is not forced if too many validators are disabled. Before this PR not all offenders were disabled. A decision was made based on [`enum DisableStrategy`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/primitives/staking/src/offence.rs#L54). Some offenders were disabled for a whole era, some just for a session, some were not disabled at all. This PR changes the disabling behaviour. Now a validator committing an offense is disabled immediately till the end of the current era. Some implementation notes: * `OffendingValidators` in pallet session keeps all offenders (this is not changed). However its type is changed from `Vec<(u32, bool)>` to `Vec<u32>`. The reason is simple - each offender is getting disabled so the bool doesn't make sense anymore. * When a validator is disabled it is first added to `OffendingValidators` and then to `DisabledValidators`. This is done in [`add_offending_validator`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/slashing.rs#L325) from staking pallet. * In [`rotate_session`](https://github.com/paritytech/polkadot-sdk/blob/bdbe98297032e21a553bf191c530690b1d591405/substrate/frame/session/src/lib.rs#L623) the `end_session` also calls [`end_era`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/pallet/impls.rs#L490) when an era ends. In this case `OffendingValidators` are cleared **(1)**. * Then in [`rotate_session`](https://github.com/paritytech/polkadot-sdk/blob/bdbe98297032e21a553bf191c530690b1d591405/substrate/frame/session/src/lib.rs#L623) `DisabledValidators` are cleared **(2)** * And finally (still in `rotate_session`) a call to [`start_session`](https://github.com/paritytech/polkadot-sdk/blob/bbb6631641f9adba30c0ee6f4d11023a424dd362/substrate/frame/staking/src/pallet/impls.rs#L430) repopulates the disabled validators **(3)**. * The reason for this complication is that session pallet knows nothing abut eras. To overcome this on each new session the disabled list is repopulated (points 2 and 3). Staking pallet knows when a new era starts so with point 1 it ensures that the offenders list is cleared. --------- Co-authored-by: ordian <noreply@reusable.software> Co-authored-by: ordian <write@reusable.software> Co-authored-by: Maciej <maciej.zyszkiewicz@parity.io> Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: command-bot <> Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>
Closes #1966, #1963 and #1962.
Disabling strategy specification here. (Updated 13/02/2024)
Implements:
validator disabling for a whole era instead of just a session(been the case even before)Removes:
DisableStrategy
enum - now each validator committing an offence is disabled.Before this PR not all offenders were disabled. A decision was made based on
enum DisableStrategy
. Some offenders were disabled for a whole era, some just for a session, some were not disabled at all.This PR changes the disabling behaviour. Now a validator committing an offense is disabled immediately till the end of the current era.
Some implementation notes:
OffendingValidators
in pallet session keeps all offenders (this is not changed). However its type is changed fromVec<(u32, bool)>
toVec<u32>
. The reason is simple - each offender is getting disabled so the bool doesn't make sense anymore.OffendingValidators
and then toDisabledValidators
. This is done inadd_offending_validator
from staking pallet.rotate_session
theend_session
also callsend_era
when an era ends. In this caseOffendingValidators
are cleared (1).rotate_session
DisabledValidators
are cleared (2)rotate_session
) a call tostart_session
repopulates the disabled validators (3).